Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: upgrade wasmd to v0.28.0 #2714

Merged
merged 11 commits into from
Sep 14, 2022
Merged

chore: upgrade wasmd to v0.28.0 #2714

merged 11 commits into from
Sep 14, 2022

Conversation

p0mvn
Copy link
Member

@p0mvn p0mvn commented Sep 12, 2022 •
edited
Loading

Part of #2566

What is the purpose of the change

Upgrade wasmd to v0.28.0

Branch: https://github.com/osmosis-labs/wasmd/tree/v0.28.0x-osmo-v12
Tag: https://github.com/osmosis-labs/wasmd/releases/tag/v0.28.0-osmo-v12.1

Testing and Verifying

This change is a trivial rework / code cleanup without any test coverage.

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? no
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no
  • How is the feature or change documented? not applicable

@github-actions github-actions bot added the C:docs Improvements or additions to documentation label Sep 12, 2022
@p0mvn p0mvn added the A:backport/v12.x backport patches to v12.x branch label Sep 12, 2022
@p0mvn
Copy link
Member Author

p0mvn commented Sep 12, 2022 •
edited
Loading

I've investigated the simulator failure, and it seems to be happening during fee validation in x/txfees:

osmosis/x/txfees/keeper/feetokens.go

Line 112 in 1d9c6e3

return types.FeeToken{}, sdkerrors.Wrapf(types.ErrInvalidFeeToken, "%s", denom)

Essentially, wasmd generates a tx with a random fee:
https://github.com/osmosis-labs/wasmd/blob/0f2794c47af3c1993f94c9f45953b3de6f4e2bf5/x/wasm/simulation/operations.go#L154

I think the solution should be to modify wasmd side and hardcode the denom to be "stake"

@p0mvn
Copy link
Member Author

p0mvn commented Sep 12, 2022

Seems that we only allow fees in "uosmo" in our simulator too:

osmosis/simulation/simtypes/legacy.go

Lines 38 to 39 in 1d9c6e3

// Only allow fees in "uosmo"
coins = sdk.NewCoins(sdk.NewCoin("uosmo", coins.AmountOf("uosmo")))

So I'm going to go ahead and make the change wasmd side and retag

@faddat
Copy link
Member

faddat commented Sep 12, 2022 •
edited
Loading

Recommend v0.29.0 in fact.

Also, I just vendored x/wasm and x/globalfees into eve, and I gotta say it feels about right, there won't be as much upstream dependency chaos that way.

I'd be happy to hack on any of that for osmo.

@github-actions github-actions bot added C:simulator Edits simulator or simulations C:x/gamm Changes, features and bugs related to the gamm module. labels Sep 13, 2022
p0mvn
p0mvn commented Sep 13, 2022
View reviewed changes
x/gamm/simulation/sim_msgs.go
@@ -355,7 +355,7 @@ func RandomExitSwapShareAmountIn(k keeper.Keeper, sim *simtypes.SimCtx, ctx sdk.
PoolId: pool_id,
TokenOutDenom: tokenOut.Denom,
ShareInAmount: gammShares.Amount.Quo(sdk.NewInt(2)),
TokenOutMinAmount: tokenOut.Amount,
TokenOutMinAmount: tokenOut.Amount.Quo(sdk.NewInt(2)),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The simulator was failing because of this. I think the calculations above are making it flaky so this change is made to unblock v12. Once the TODO above is addressed, this should be as well

@p0mvn
Copy link
Member Author

p0mvn commented Sep 13, 2022

Recommend v0.29.0 in fact.

Also, I just vendored x/wasm and x/globalfees into eve, and I gotta say it feels about right, there won't be as much upstream dependency chaos that way.

I'd be happy to hack on any of that for osmo.

v0.29.0 is still an RC. Also, we really want to get v12 out, and this is one of the last missing pieces.

Since regular wasmd tag requires additional changes to be compatible with Osmosis, I think we will go with v0.28 for the sake of delivering v12. However, happy to upgrade to v0.29 as soon as stable release is out.

Also, I just vendored x/wasm and x/globalfees into eve, and I gotta say it feels about right

Do you mean added it directly into the repository? How are you planning to integrate improvements from upstream?

@p0mvn p0mvn added this to the V12 Blockers milestone Sep 14, 2022
@p0mvn p0mvn marked this pull request as ready for review September 14, 2022 04:38
@p0mvn p0mvn requested a review from a team September 14, 2022 04:38
@p0mvn p0mvn merged commit b1c92ae into main Sep 14, 2022
@p0mvn p0mvn deleted the roman/upgrade-wasmd branch September 14, 2022 18:16
mergify bot pushed a commit that referenced this pull request Sep 14, 2022
@p0mvn @mergify
chore: upgrade wasmd to v0.28.0 (#2714)
e85becd 
* chore: upgrade wasmd to v0.28.0

* changelog

* fix simulator

* update dependency

* simulator time hack

* update simulator

* remove curTime

* update tag

* update comments

* updates

* update wasmd dep

(cherry picked from commit b1c92ae)
@mergify mergify bot mentioned this pull request Sep 14, 2022
Merged
p0mvn added a commit that referenced this pull request Sep 14, 2022
@mergify @p0mvn
chore: upgrade wasmd to v0.28.0 (backport #2714) (#2744)
d7ed36f 
* chore: upgrade wasmd to v0.28.0 (#2714)

* chore: upgrade wasmd to v0.28.0

* changelog

* fix simulator

* update dependency

* simulator time hack

* update simulator

* remove curTime

* update tag

* update comments

* updates

* update wasmd dep

(cherry picked from commit b1c92ae)

* updates

Co-authored-by: Roman <[email protected]>
Co-authored-by: Roman <[email protected]>
@github-actions github-actions bot mentioned this pull request Feb 1, 2024
Closed
@github-actions github-actions bot mentioned this pull request Oct 15, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@czarcas7ic czarcas7ic czarcas7ic approved these changes

Assignees
No one assigned
Labels
A:backport/v12.x backport patches to v12.x branch C:docs Improvements or additions to documentation C:simulator Edits simulator or simulations C:x/gamm Changes, features and bugs related to the gamm module.
Projects
Osmosis Chain Development
Archived in project
Milestone
V12 Blockers
Development

Successfully merging this pull request may close these issues.
3 participants
@p0mvn @faddat @czarcas7ic